Improved handling of --time-limit when combined with -J

When concurrency is enabled, there can be worker threads still running
when the time limit is checked. Exiting right there does not
give those threads time to finish what they're doing. Instead, the seeking
is wrapped up, and git-annex then shuts down cleanly.

The whole point of --time-limit existing, rather than using timeout(1)
when running git-annex is to let git-annex finish the action(s) it is
working on when the time limit is reached, and shut down cleanly.

I noticed this problem when investigating why restagePointerFile might
not have run after get/drop of an unlocked file. With --time-limit -J,
a worker thread may have finished updating a work tree file, and be killed
by the time limit check before it can run restagePointerFile. So despite
--time-limit running the shutdown actions, the work tree file didn't get
restaged.

Sponsored-by: Dartmouth College's DANDI project

diff --git a/Annex.hs b/Annex.hs
index 6e4ebb9b7087ef234246584c420ef5f3549403ca..0f0464dcacf12f9634cecaa2827ccdcec3edf3f9 100644 (file)
--- a/Annex.hs
+++ b/Annex.hs
@@ -200,7 +200,7 @@ data AnnexState = AnnexState
        , cleanupactions :: M.Map CleanupAction (Annex ())
        , sentinalstatus :: Maybe SentinalStatus
        , errcounter :: Integer
-       , skippedfiles :: Bool
+       , reachedlimit :: Bool
        , adjustedbranchrefreshcounter :: Integer
        , unusedkeys :: Maybe (S.Set Key)
        , tempurls :: M.Map Key URLString
@@ -253,7 +253,7 @@ newAnnexState c r = do
                , cleanupactions = M.empty
                , sentinalstatus = Nothing
                , errcounter = 0
-               , skippedfiles = False
+               , reachedlimit = False
                , adjustedbranchrefreshcounter = 0
                , unusedkeys = Nothing
                , tempurls = M.empty

diff --git a/CHANGELOG b/CHANGELOG
index 0623c8b6e7d31f1b490cea2ab450d85e4f6e87a5..eebd73c927ab90d33e6b0b0a39d7fa65d0337b66 100644 (file)
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -28,6 +28,7 @@ git-annex (10.20220823) UNRELEASED; urgency=medium
     repository when --git-dir or GIT_DIR is specified to relocate the git
     directory to somewhere else.
     (Introduced in version 10.20220525)
+  * Improved handling of --time-limit when combined with -J
 
  -- Joey Hess <id@joeyh.name>  Mon, 29 Aug 2022 15:03:04 -0400
 

diff --git a/CmdLine/Action.hs b/CmdLine/Action.hs
index 6d7932bb0bb98046578f47c3e344cbe1e819d23c..d99cdad17a6f74940bb209dac502d369dfb71a82 100644 (file)
--- a/CmdLine/Action.hs
+++ b/CmdLine/Action.hs
@@ -32,8 +32,8 @@ import qualified System.Console.Regions as Regions
 {- Runs a command, starting with the check stage, and then
  - the seek stage. Finishes by running the continuation.
  -
- - Can exit when there was a problem or when files were skipped.
- - Also shows a count of any failures when that is enabled.
+ - Can exit when there was a problem or when a time or size limit was
+ - reached. Also shows a count of any failures when that is enabled.
  -}
 performCommandAction :: Bool -> Command -> CommandSeek -> Annex () -> Annex ()
 performCommandAction canexit (Command { cmdcheck = c, cmdname = name }) seek cont = do
@@ -43,19 +43,19 @@ performCommandAction canexit (Command { cmdcheck = c, cmdname = name }) seek con
        finishCommandActions
        cont
        st <- Annex.getState id
-       when canexit $ liftIO $ case (Annex.errcounter st, Annex.skippedfiles st) of
+       when canexit $ liftIO $ case (Annex.errcounter st, Annex.reachedlimit st) of
                (0, False) -> noop
                (errcnt, False) -> do
                        showerrcount errcnt
                        exitWith $ ExitFailure 1
-               (0, True) -> exitskipped
+               (0, True) -> exitreachedlimit
                (errcnt, True) -> do
                        showerrcount errcnt
-                       exitskipped
+                       exitreachedlimit
   where
        showerrcount cnt = hPutStrLn stderr $
                name ++ ": " ++ show cnt ++ " failed"
-       exitskipped = exitWith $ ExitFailure 101
+       exitreachedlimit = exitWith $ ExitFailure 101
 
 commandActions :: [CommandStart] -> Annex ()
 commandActions = mapM_ commandAction
@@ -328,7 +328,7 @@ checkSizeLimit (Just sizelimitvar) startmsg a =
                        Nothing -> do
                                fsz <- catchMaybeIO $ withObjectLoc k $
                                        liftIO . getFileSize
-                               maybe skipped go fsz
+                               maybe reachedlimit go fsz
                Nothing -> a
   where
        go sz = do
@@ -342,6 +342,6 @@ checkSizeLimit (Just sizelimitvar) startmsg a =
                                else return False
                if fits 
                        then a
-                       else skipped
+                       else reachedlimit
        
-       skipped = Annex.changeState $ \s -> s { Annex.skippedfiles = True }
+       reachedlimit = Annex.changeState $ \s -> s { Annex.reachedlimit = True }

diff --git a/CmdLine/Seek.hs b/CmdLine/Seek.hs
index 24ee8f03f4fbf98c3eade13c43915ac653e4f74e..895250ec8d9d9c0c4cc1e1c4ef547a743ad0d29b 100644 (file)
--- a/CmdLine/Seek.hs
+++ b/CmdLine/Seek.hs
@@ -4,7 +4,7 @@
  - the values a user passes to a command, and prepare actions operating
  - on them.
  -
- - Copyright 2010-2021 Joey Hess <id@joeyh.name>
+ - Copyright 2010-2022 Joey Hess <id@joeyh.name>
  -
  - Licensed under the GNU AGPL version 3 or higher.
  -}
@@ -41,7 +41,6 @@ import Annex.Link
 import Annex.InodeSentinal
 import Annex.Concurrent
 import Annex.CheckIgnore
-import Annex.Action
 import qualified Annex.Branch
 import qualified Database.Keys
 import qualified Utility.RawFilePath as R
@@ -49,6 +48,7 @@ import Utility.Tuple
 import Utility.HumanTime
 
 import Control.Concurrent.Async
+import Control.Concurrent.STM
 import System.Posix.Types
 import Data.IORef
 import Data.Time.Clock.POSIX
@@ -610,20 +610,25 @@ notSymlink :: RawFilePath -> IO Bool
 notSymlink f = liftIO $ not . isSymbolicLink <$> R.getSymbolicLinkStatus f
 
 {- Returns an action that, when there's a time limit, can be used
- - to check it before processing a file. The first action is run when over the
- - time limit, otherwise the second action is run. -}
+ - to check it before processing a file. The first action is run when
+ - over the time limit, otherwise the second action is run one time to
+ - clean up. -}
 mkCheckTimeLimit :: Annex (Annex () -> Annex () -> Annex ())
 mkCheckTimeLimit = Annex.getState Annex.timelimit >>= \case
        Nothing -> return $ \_ a -> a
-       Just (duration, cutoff) -> return $ \cleanup a -> do
-               now <- liftIO getPOSIXTime
-               if now > cutoff
-                       then do
-                               warning $ "Time limit (" ++ fromDuration duration ++ ") reached! Shutting down..."
-                               shutdown True
-                               cleanup
-                               liftIO $ exitWith $ ExitFailure 101
-                       else a
+       Just (duration, cutoff) -> do
+               warningshownv <- liftIO $ newTVarIO False
+               return $ \cleanup a -> do
+                       now <- liftIO getPOSIXTime
+                       if now > cutoff
+                               then do
+                                       warningshown <- liftIO $ atomically $
+                                               swapTVar warningshownv True
+                                       unless warningshown $ do
+                                               Annex.changeState $ \s -> s { Annex.reachedlimit = True }
+                                               warning $ "Time limit (" ++ fromDuration duration ++ ") reached! Shutting down..."
+                                               cleanup
+                               else a
 
 propagateLsFilesError :: IO Bool -> Annex ()
 propagateLsFilesError cleanup =

diff --git a/doc/git-annex-common-options.mdwn b/doc/git-annex-common-options.mdwn
index 07054960353ab8f5d0b3911ffd59eb02a0bc8341..932d7288c15b28e8bf3b32887aedae042bff22b5 100644 (file)
--- a/doc/git-annex-common-options.mdwn
+++ b/doc/git-annex-common-options.mdwn
@@ -68,8 +68,10 @@ Most of these options are accepted by all git-annex commands.
   Limits how long a git-annex command runs. The time can be something
   like "5h", or "30m" or even "45s" or "10d".
 
-  Note that git-annex may continue running a little past the specified
-  time limit, in order to finish processing a file.
+  Note that git-annex may continue running for some time past the specified
+  time limit, in order to finish processing files it started before the
+  time limit was reached. That and a cleaner shutdown are the differences
+  between using this option and a command like `timeout(1)`.
 
   When the time limit prevents git-annex from doing all it
   was asked to, it will exit with a special code, 101.